Redirect old theme_page Site Editor routes to the new site-editor.php page
#42643
Conversation
|
I'll amend and rename the |
fullofcaffeine
force-pushed
the
add/backwards-compatibility-to-site-editor-routes
branch
from
341550f
to
6b1207b
Compare
There was a problem hiding this comment.
I'm wondering how third party plugins handle their dependencies and what was the goal by using the GB url. Was there the need for adding some links to site editor? Did they made sure that GB was installed and active? Are there checks about the min version of WP or GB supported?
I'm not sure if this is something that should be handled in GB or the plugins that used that, but if we end up needing to keep the redirects, I think it should be commented to be for a limited time to give time to plugins to update their code..
Unfortunately, I don't know, and I think that given the open nature of the WP ecosystem, it'll be hard to know for sure. I still think that a route change should be done with backward compatibility in mind, similar to the standard way of deprecating API routes -- i.e keep the old route so as not to break existing consumers. In this case, we possibly wouldn't need to keep the redirect forever, but only as a transient solution while consumers update their code to handle the route changing. The definitive solution could be to update the code for consumers to handle the new route. That will depend on changes in upstream code for any third-party plugins that still use the old routes. This can't be a solution now as we don't have a full list of all the plugins that are affected, so this change will act as a safety-net while upstream devs work on the fix (hopefully), and this will take time (to be honest, though, I still think we should somehow abstract routes to prevent this from happening in the future, see the PR description). One thing is for sure: we will need to support GB < 13.7 so we need to keep supporting the old routes and now with 13.7, the new route. |
fullofcaffeine
force-pushed
the
add/backwards-compatibility-to-site-editor-routes
branch
from
6b1207b
to
551d3a1
Compare
fullofcaffeine
force-pushed
the
add/backwards-compatibility-to-site-editor-routes
branch
from
551d3a1
to
fb817a7
Compare
Co-authored-by: George Mamadashvili <[email protected]>
Co-authored-by: Andrei Draganescu <[email protected]>
Co-authored-by: George Mamadashvili <[email protected]>
|
Thanks, folks! |
…php` page (#42643) * Redirect pre-13.7 Site Editor routes to `site.editor.php` * move file to different compat folder * update file structure docs * Update lib/compat/plugin/edit-site-routes-backwards-compat.php Co-authored-by: George Mamadashvili <[email protected]> * fix empty rendered menu item * Apply suggestions from code review Co-authored-by: Andrei Draganescu <[email protected]> * Update lib/compat/plugin/edit-site-routes-backwards-compat.php Co-authored-by: George Mamadashvili <[email protected]> Co-authored-by: ntsekouras <[email protected]> Co-authored-by: George Mamadashvili <[email protected]> Co-authored-by: Andrei Draganescu <[email protected]>
What?
Redirect the following routes:
admin.php?page=gutenberg-edit-sitethemes.php?page=gutenberg-edit-siteTo:
site-editor.phpWhy?
Gutenberg 13.7 deprecated the
theme_page-based routes (see above) for the Site Editor in this PR: #41306. However, third-party code (i.e from plugins -- ex: Jetpack) might still reference the old routes. We don't want their flows to break with the 13.7 upgrade, and a good transient solution is to keep the old routes around while making them redirect to the new route.What about the definitive solution?
I don't know what is the current backwards-compatibility policy for WordPress/Gutenberg, but it appears that we should be more careful about deprecating routes/paths. Ideally, routes are abstracted in helper methods and not hardcoded throughout the codebase. For example, let's say that the Site Editor route was wrapped in a
site_editor_urlhelper method from the beginning. The helper would be defined in GB or WP version (if backported). Third-party consumers would (hopefully) be using that, and any underlying changes would be picked up automatically. This is more of food for thought for future route additions. Unfortunately, for the Site Editor, adding the helper now won't solve the immediate problem.To entirely deprecate the old paths and get rid of the redirect, third-party consumers would need to add a conditional to check for the GB version and potentially the WP version, too (since the code might be backported to WP in the future, AFAIK). This is a possibility, and the effort would depend on how many third-party consumers are already referencing the old paths.
Or we could add this redirect and then add a route abstraction as suggested above so that any future changes won't need any conditionals or redirects anymore.
What are your thoughts?
How?
Adds a new "theme_page" if the theme is block_theme. This seems to whitelist the route and allows it to be accessible. Without it, we get a permission error from WordPress. It's added as a callback to the
admin_menuaction.Then, we add a callback to the
load-appearance_page_gutenberg-edit-siteaction that finally redirects. From what I understand, this action is triggered when the aforementioned "theme_page" is loaded, and then the redirect takes place.There's a problem for now, though, which I'm still trying to solve: the
add_theme_pageadds a new submenu for the Editor. We end up with two items under appearance :( ... is there a way to whitelist the routes without usingadd_theme_page?I mainly found the code that worked by trial and error, so I would appreciate thoughts about whether there are any other better ways to accomplish it.
Testing Instructions
wp-env, login to the WP instance;localhost:8889/wp-admin/admin.php?page=gutenberg-edit-siteshould redirect tolocalhost:8889/wp-admin/site-editor.phplocalhost:8889/wp-admin/themes.php?page=gutenberg-edit-siteshould redirect tolocalhost:8889/wp-admin/admin/site-editor.phpScreenshots or screencast